Provision RBAC for Kubeconfigs inside kcp#91
Conversation
kcp-ci-bot
added
do-not-merge/work-in-progress
xrstf
force-pushed
the
kubeconfig-authz
branch
from
600c366 to
0a36122
Compare
kcp-ci-bot
added
the
needs-rebase
xrstf
force-pushed
the
kubeconfig-authz
branch
from
d14cd21 to
99f7ce4
Compare
kcp-ci-bot
added
size/XXL
xrstf
force-pushed
the
kubeconfig-authz
branch
from
c4d020c to
6fbcf98
Compare
kcp-ci-bot
added
release-note
|
/kind feature |
kcp-ci-bot
added
kind/feature
|
/retest |
| clusterRoles: | ||
| items: | ||
| type: string | ||
| type: array |
There was a problem hiding this comment.
Does clusterRoles only reference objects that already exist in the cluster? I am wondering if it could be made possible to also configure RBAC inside the cluster?
There was a problem hiding this comment.
For now this is only binding to pre-existing ClusterRoles, yes.
|
/retest |
4 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
xrstf
changed the title
kcp-ci-bot
removed
the
do-not-merge/work-in-progress
|
/merge-method squash |
xrstf
added
the
tide/merge-method-squash
…ervice inside kind On-behalf-of: @SAP [email protected]
On-behalf-of: @SAP [email protected]
On-behalf-of: @SAP [email protected]
On-behalf-of: @SAP [email protected]
On-behalf-of: @SAP [email protected]
On-behalf-of: @SAP [email protected]
On-behalf-of: @SAP [email protected]
On-behalf-of: @SAP [email protected]
xrstf
force-pushed
the
kubeconfig-authz
branch
from
9919352 to
ab42d4a
Compare
kcp-ci-bot
removed
the
needs-rebase
On-behalf-of: @SAP [email protected]
|
@mjudeikis PTAL :) |
mjudeikis
left a comment
mjudeikis
left a comment
There was a problem hiding this comment.
few nits. I still dont like the fact its public functions, pointers and we hope users will do the right thing without single code comments on the functions "what is the right thing" :D
internal/client/clients.go
Outdated
| // only one of these three should be provided, the others nil | ||
| rootShard *operatorv1alpha1.RootShard, | ||
| shard *operatorv1alpha1.Shard, |
There was a problem hiding this comment.
I think its a lie. They are called from public function where they are pointers and there is no nil checking. Either add checking or make them non-pointers
There was a problem hiding this comment.
Kube objects are basically in 99.9999% of cases kept on the heap and dereferencing them will just lead to annoying-to-use variables.
But I've added extra nil checks, even though they are validated in getRootShard(). :)
There was a problem hiding this comment.
I've refactored the code now to get rid of this triple parameter.
On-behalf-of: @SAP [email protected]
On-behalf-of: @SAP [email protected]
|
LGTM label has been added. DetailsGit tree hash: e97c507f6cae281e2fb24528ef72895f10908486 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mjudeikis The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kcp-ci-bot
added
the
approved
5 participants
Summary
This PR implements a new feature for the operator: It can now provision RBAC inside kcp and thereby grant people permissions and take them away, too.
This is now the first PR to make use of the internal-proxy (#87): since admins can configure any random workspace path or cluster name, the operator needs to be able to provision on any shard and more importantly, figure out which shard. To solve this, we use our internal proxy ("internal" still means a standalone Deployment, of course).
Each Kubeconfig object can now hold a workspace and a desired list of permissions inside that workspace. The operator will try to reconcile these RBAC resources accordingly, and also take care of cleaning up when a Kubeconfig is removed or changed (it's possible for users to change the workspace that RBAC should be placed in, and the operator will first cleanup the old cluster and then provision the new one).
To keep track of where RBAC has been deployed, a new field in the Kubeconfig status has been introduced. We discussed this and decided that this is a safe place to do so, as anyone with permissions to manage Kubeconfigs is technically an admin and so endusers cannot/should not fiddle with Kubeconfigs. If that were possible, the operator currently has no way of defending against malicious changes.
Each kubeconfig manages its own RBAC and all resources inside kcp are named based on the UID of the Kubeconfig object. This ensures uniqueness all around and avoids having to merge desired RBACs into one ClusterRole(Binding) and unfiddle them when RBAC for one Kubeconfig is removed.
Notably, since the kcp-operator now has to talk with shards and the front-proxy, this PR modifies the local e2e setup to work like the CI e2e test: build an operator image and deploy it into kind, rather than running the operator on the host machine. This is a bit sad for quick debugging tests, but saves us from somehow having to either dynamically expose the pods through kind to the host, or rewriting URLs in the operator somehow.
What Type of PR Is This?
/kind feature
Related Issue(s)
Fixes #49
Release Notes